Skip to content

feat(telegram): first-class [telegram] config section#1265

Merged
chaodu-agent merged 10 commits into
mainfrom
feat/telegram-first-class-config
Jun 30, 2026
Merged

feat(telegram): first-class [telegram] config section#1265
chaodu-agent merged 10 commits into
mainfrom
feat/telegram-first-class-config

Conversation

@chaodu-agent

Copy link
Copy Markdown
Collaborator

Implements the first-class per-platform config ADR (#1263) for Telegram: every TELEGRAM_* env var now has a corresponding field under a top-level [telegram] section, symmetric with [discord] / [slack].

Precedence (per field)

[telegram] value (with ${} expansion) → TELEGRAM_* env var → built-in default.

Config-authoritative, matching OpenAB's existing [discord]/[slack] convention. Backward compatible — env-only deployments (no [telegram] section) are unchanged; any omitted field falls back to its env var.

Field mapping

[telegram] field Env fallback Default
bot_token TELEGRAM_BOT_TOKEN
secret_token TELEGRAM_SECRET_TOKEN
trusted_source_only TELEGRAM_TRUSTED_SOURCE_ONLY false
rich_messages TELEGRAM_RICH_MESSAGES true
streaming TELEGRAM_STREAMING follows rich_messages
webhook_path TELEGRAM_WEBHOOK_PATH /webhook/telegram

Changes

  • openab-core/config.rs: TelegramConfig + resolve() (config > env > default), telegram field on Config
  • openab-gateway/lib.rs: AppState.telegram_streaming field + apply_telegram_config() (plain params — no core dependency); both from_env and serve() initializers read TELEGRAM_STREAMING
  • unified_adapter.rs: use_streaming() reads the resolved field instead of std::env::var at runtime
  • main.rs: embedded gateway applies resolved [telegram] config + webhook path
  • Docs: docs/telegram.md + config.toml.example

Scope

Covers the TELEGRAM_* transport/platform vars (this issue's ask). Per-platform trust fields (allowed_users, allow_dm, …) land with the identity-trust-none work (#1264).

Testing

  • 3 new unit tests (config-wins, env-fallback + defaults + legacy bool semantics, TOML parse)
  • cargo clippy --features unified -D warnings clean
  • full openab-core suite passes (the one unrelated secrets::resolve_exec_nonzero_exit failure is a pre-existing macOS-only quirk, green on Linux CI)

Refs #1263

Implements #1263 for Telegram: every TELEGRAM_* env var now has a
[telegram] config field. Config-authoritative with ${} expansion and
TELEGRAM_* env fallback when a field is unset (matches [discord]/[slack]).

- TelegramConfig + resolve() in openab-core (config > env > default)
- AppState gains telegram_streaming + apply_telegram_config() (plain
  params, no core dependency)
- unified_adapter reads resolved streaming instead of env directly
- main.rs applies resolved config + webhook_path to the embedded gateway
- backward compatible: env-only deployments (no [telegram]) unchanged

Refs #1263
@chaodu-agent chaodu-agent requested a review from thepagent as a code owner June 30, 2026 21:08
@chaodu-agent

This comment has been minimized.

@chaodu-agent

This comment has been minimized.

…egramConfig param object

- Filter empty strings in TelegramConfig::resolve() so ${UNSET_VAR}
  expansion to "" correctly falls through to TELEGRAM_* env fallback
  instead of holding Some("") that bypasses the fallback chain.
- Replace 5-param apply_telegram_config() with GatewayTelegramConfig
  parameter object for a clean, extensible cross-crate API boundary.
- Add test: telegram_empty_string_falls_through_to_env
@chaodu-agent

This comment has been minimized.

…ting tests

- Startup validation now recognizes [telegram] section as a valid
  adapter config (no longer requires TELEGRAM_BOT_TOKEN env var).
- Unified server spawn condition also triggers on cfg.telegram.is_some(),
  so config-only deployments work as promised by the PR.
- Merge empty-string test into the single serialized
  telegram_env_fallback_and_defaults test to eliminate env var race
  conditions under parallel test execution.
@chaodu-agent

This comment has been minimized.

…test

Merge telegram_config_value_wins_over_env into
telegram_resolve_all_scenarios (renamed from telegram_env_fallback_and_defaults)
to eliminate any possible env var race condition under parallel test execution.
All TELEGRAM_* env mutations are now in one #[test] function.
@chaodu-agent

This comment has been minimized.

@chaodu-agent

This comment has been minimized.

@chaodu-agent

This comment has been minimized.

@chaodu-agent

This comment has been minimized.

@chaodu-agent

This comment has been minimized.

@chaodu-agent

This comment has been minimized.

@chaodu-agent

Copy link
Copy Markdown
Collaborator Author

LGTM ✅ — First-class [telegram] config section with correct precedence chain, clean crate decoupling, full backward compatibility, and config-only deployment support.

What This PR Does

Adds a first-class [telegram] configuration section to config.toml, implementing the per-platform config ADR (#1263) for Telegram. Every TELEGRAM_* env var now has a corresponding config field with config-authoritative precedence: [telegram] value (with ${} expansion) → TELEGRAM_* env var → built-in default.

Minimal required config:

[telegram]
bot_token = "${TELEGRAM_BOT_TOKEN}"    # or use aws-sm:// secret ref

How It Works

  • TelegramConfig struct with resolve() method in openab-core handles three-layer resolution, filtering empty strings from ${} expansion of unset vars.
  • GatewayTelegramConfig parameter object bridges the crate boundary — openab-gateway stays free of an openab-core dependency.
  • unified_adapter.rs reads the resolved telegram_streaming field from AppState at startup instead of per-request std::env::var calls (eliminates TOCTOU).
  • Startup validation and unified server spawn both recognize [telegram] as a valid activation trigger, so pure config-only deployments work without any TELEGRAM_* env vars.

Findings

# Severity Finding Location
1 🟢 Correct three-layer precedence with empty-string filtering for ${} expansion edge case config.rs:620-660
2 🟢 Clean crate-boundary decoupling via GatewayTelegramConfig parameter object lib.rs:204-220
3 🟢 Config-only activation path works end-to-end: validation → spawn → config apply → webhook mount main.rs:200,501
4 🟢 use_streaming() reads startup-resolved field instead of per-call env::var — eliminates TOCTOU unified_adapter.rs:204
5 🟢 All env-mutating tests consolidated into single #[test] — no race conditions config.rs tests
6 🟢 Legacy bool semantics preserved — env_flag_true_one (opt-in) vs env_flag_not_false (opt-out) config.rs:660-680
7 🟢 Security hardening documented — aws-sm:// SecretRef recommended for production docs/telegram.md
What's Good (🟢)
  • Backward compatible by design: env-only deployments (no [telegram] section) work identically — TelegramConfig::default().resolve() produces the same results as previous env-only code paths.
  • Config-only deployments supported: cfg.telegram.is_some() in both startup validation and unified spawn means operators can use [telegram] in config.toml without any TELEGRAM_* env vars.
  • Empty-string guard: .filter(|s| !s.is_empty()) on bot_token, secret_token, and webhook_path handles the edge case where ${UNSET_VAR} expands to "" at parse time — falls through to env correctly.
  • Clean crate separation: GatewayTelegramConfig avoids coupling openab-gateway to openab-core, keeping the crate graph clean and extensible.
  • Race-free tests: all env-mutating scenarios consolidated into one #[test] function (telegram_resolve_all_scenarios), eliminating parallel test flakiness.
  • Runtime efficiency: streaming decision resolved once at startup rather than per-request env lookups.
  • Security guidance: docs recommend aws-sm:// secret references for production, with a complete example using [secrets.refs].
  • Comprehensive documentation: minimal config, full config, precedence table, config-only tip, and security hardening all covered in docs/telegram.md.
Review History

This review incorporates findings from 3 rounds of team review:

Round 1 — identified empty-string ${} expansion bug and too-many-arguments code smell.
Round 2 — found critical bug: [telegram] config-only deployments failed because startup validation and unified spawn only checked env vars. Also flagged test env-var race conditions.
Round 3 — all findings fixed and verified. Config-only activation, empty-string filtering, parameter object, test consolidation, docs updates all confirmed correct.

All 🔴 and 🟡 findings have been addressed in follow-up commits on this branch.

Baseline Check
  • PR opened: 2026-06-30
  • Main already has: TELEGRAM_* env vars read directly at startup in AppState::from_env() and serve(), plus runtime std::env::var("TELEGRAM_STREAMING") in use_streaming()
  • Net-new value: First-class [telegram] config section with typed resolution, config-authoritative precedence, startup-time resolution (no per-request env lookups), config-only deployment support, GatewayTelegramConfig bridge type, security hardening guidance, and comprehensive documentation

@chaodu-agent chaodu-agent enabled auto-merge June 30, 2026 22:05
@chaodu-agent chaodu-agent merged commit b993048 into main Jun 30, 2026
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants